Migrate BotBuilder SDK to Teams SDK bot-adaptive-card-actions sample #1844
Conversation
There was a problem hiding this comment.
use minimal APIs instead of controllers
There was a problem hiding this comment.
We are following standard Template created by TeamsToolkit. Controller has been used in all standard templates.
There was a problem hiding this comment.
we are going to deprecate Controllers. Please use Minimal APIs instead
There was a problem hiding this comment.
Updated code to use minimal APIs and removed the controllers
| /// <summary> | ||
| /// Removes bot mention from the message text. | ||
| /// </summary> | ||
| private string RemoveBotMention(string text) |
There was a problem hiding this comment.
Removed the custom Function RemoveBotMention.
| } | ||
|
|
||
| // Loads and sends an adaptive card from the specified JSON file | ||
| private async Task SendAdaptiveCardAsync(IContext.Client client, string cardFileName, string userName = null) |
There was a problem hiding this comment.
Updated code to use Cards API.
| } | ||
| else | ||
| { | ||
| await client.Send(CommandString); |
There was a problem hiding this comment.
we should pass the cancellationToken to all async functions
There was a problem hiding this comment.
.Send() method accepts only 2 arguments (string , bool), It doesn't support cancellationToken.
There was a problem hiding this comment.
oh.. you are right. Can you file an issue in teams.net ?
There was a problem hiding this comment.
this file should be .gitignored, and add a launchSetting.example.json as a reference
There was a problem hiding this comment.
Added this file in .gitignore , also added launchSetting.example.json as a reference
| <LangVersion>latest</LangVersion> | ||
| <TargetFramework>net10.0</TargetFramework> | ||
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <LangVersion>14.0</LangVersion> |
There was a problem hiding this comment.
any reason to use LangVersion?
We should enable nullable support
There was a problem hiding this comment.
Removed LangVersion and added Nullable support.
| <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="6.0.11" /> | ||
| <PackageReference Include="Microsoft.Bot.Builder.Integration.AspNet.Core" Version="4.18.1" /> | ||
| <PackageReference Include="Azure.Identity" Version="1.13.1" /> | ||
| <PackageReference Include="Microsoft.Teams.Api" Version="2.0.*" /> |
There was a problem hiding this comment.
Updated and only kept by default created packages
There was a problem hiding this comment.
you only need Plugins.AspNetCore, all the other packages are transitive dependencies and dont need to be in the csproj
| <PackageReference Include="Microsoft.Teams.Plugins.AspNetCore" Version="2.0.*" /> | ||
| <PackageReference Include="Microsoft.Teams.Common" Version="2.0.*" /> | ||
| <PackageReference Include="AdaptiveCards.Templating" Version="1.6.0" /> | ||
| <PackageReference Include="Newtonsoft.Json" Version="13.0.3" /> |
There was a problem hiding this comment.
any reason to explicitly reference newtonsoft?
| </ItemGroup> | ||
|
|
||
| <!-- Exclude old Bot Builder files from compilation --> | ||
| <ItemGroup> |
|
|
||
| <!-- Exclude generated PolySharp files --> | ||
| <ItemGroup> | ||
| <Compile Remove="obj\**\PolySharp.SourceGenerators\**\*.g.cs" /> |
There was a problem hiding this comment.
how is this used? I'm not aware of the need to use PolySharp
| </ItemGroup> | ||
|
|
||
| <!-- Exclude local settings from publish --> | ||
| <ItemGroup> |
| "Teams": { | ||
| "ClientId": "", | ||
| "ClientSecret": "", | ||
| "BotType": "", |
There was a problem hiding this comment.
what is BotType? I dot not recognize this setting
There was a problem hiding this comment.
Removed all BotType references.
| @@ -0,0 +1,19 @@ | |||
| <Project> | |||
| <PropertyGroup> | |||
| <PolySharpIncludeGeneratedTypes> | |||
There was a problem hiding this comment.
Removed the file.
| var config = builder.Configuration.Get<ConfigOptions>(); | ||
|
|
||
| // Factory function to create access tokens using managed identity credentials | ||
| Func<string[], string?, Task<ITokenResponse>> createTokenFactory = async (string[] scopes, string? tenantId) => |
There was a problem hiding this comment.
once we merge MSAL support, we won't need this.
There was a problem hiding this comment.
Removed the fuction.
There was a problem hiding this comment.
this file is usually gitignored.. what's the reason to commit it?
There was a problem hiding this comment.
This file is required, if we remove this , these options will not be visible.
| > NOTE: When you create your app registration, you will create an App ID and App password - make sure you keep these for later. | ||
|
|
||
| 2. Run ngrok - point to port 3978 | ||
| 2. Run ngrok - point to port 5130 |
There was a problem hiding this comment.
do not lead with ngrok, we should prioritize devtunnels
There was a problem hiding this comment.
Updated the README to prioritize dev tunnels over ngrok.
| * Leave **Redirect URI** empty. | ||
| * Choose **Register**. | ||
| B) On the overview page, copy and save the **Application (client) ID, Directory (tenant) ID**. You'll need those later when updating your Teams application manifest and in the appsettings.json. | ||
| B) On the overview page, copy and save the **Application (client) ID, Directory (tenant) ID**. You'll need those later when updating your Teams application manifest and in the appsettings.Development.json. |
There was a problem hiding this comment.
in which case do we need the TenantID in the manifest?
appSettings.Development.json is just one way of providing values to IConfiguration. Instead of this file, I'd recommend launchSettings.json
There was a problem hiding this comment.
Updated Readme.
| - Modify the `/appsettings.Development.json` and fill in the following details: | ||
| - `ClientId` - Generated from Step 1 is the application app id | ||
| - `ClientSecret` - Generated from Step 1, also referred to as Client secret | ||
| - `BotType` - Set the bot type (e.g., "MultiTenant" or "SingleTenant") |
There was a problem hiding this comment.
Removed BotType entirely.
| <PackageReference Include="Azure.Identity" Version="1.13.1" /> | ||
| <PackageReference Include="Microsoft.Teams.Api" Version="2.0.*" /> | ||
| <PackageReference Include="Microsoft.Teams.Apps" Version="2.0.*" /> | ||
| <PackageReference Include="Microsoft.Teams.Plugins.AspNetCore" Version="2.0.*" /> |
There was a problem hiding this comment.
Updated and only kept by default created packages and moved sample to different PR:
#1853
| <ItemGroup> | ||
| <Content Update="appsettings.json"> | ||
| <CopyToOutputDirectory>Always</CopyToOutputDirectory> | ||
| <Content Remove="appsettings.Development.json" /> |
There was a problem hiding this comment.
They are by default created.
| if (member.Id != activity.Recipient.Id) | ||
| { | ||
| await client.Send("Welcome to the suggested actions bot. This bot will introduce you to suggested actions. Please answer the question:"); | ||
| await SendSuggestedActionsCardAsync(client, log); |
There was a problem hiding this comment.
.Send() method accepts only 2 arguments (string , bool), It doesn't support cancellationToken.
| "commandName": "Project", | ||
| "dotnetRunMessages": true, | ||
| "applicationUrl": "https://localhost:7130;http://localhost:5130", | ||
| "applicationUrl": "http://localhost:5130", |
There was a problem hiding this comment.
Updated to 3978 and moved Sample to different PR: #1854
There was a problem hiding this comment.
having one action per sample/language will lead to 202 actions. We should have only one, or one per language max.
There was a problem hiding this comment.
Deleted the file, we will update one single file.
This reverts commit 0030343.
|
|
||
| # User-specific files | ||
| *.user | ||
| launchSettings.json |
There was a problem hiding this comment.
can you .gitignore launchSettings.json from the top .gitignore? it will be applicable to all samples
There was a problem hiding this comment.
Added launchSettings.json in root level .gitgnore
YugalPradhan31
changed the title
YugalPradhan31
changed the title
Harikrishnan-MSFT
changed the title
No description provided.